Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add callbackUrl option #25

Closed

Conversation

arthur-pinner
Copy link
Contributor

@arthur-pinner arthur-pinner commented Oct 10, 2023

Description

This PR implements a fix for this issue

Add a new customParams.callbackUrl option to TranslationsTabConfigOptions to allow apps to inform a callback url to be called after the content has been translated in Smartling.
You can read about more this parameter for Smartling here

Change

Smartling allows to inform a callback url at 2 different moments:

  • When we create the job. It's called when the job has been finished.
  • When we upload a file to a batch. It's called when the translation for that file and locale has been finished.

I made the call to set the callback url while uploading the file to a batch. Why?

  • Allow to have a callback by language
  • Sometimes the content is not authorized to be translated to all locales, only a few and then the job never gets done.
  • Sometimes some locales has priority over other locales to be translated.

Related PR's:

@@ -41,7 +41,7 @@ export const findExistingJob = async (
'The Smartling adapter requires a Smartling project identifier and a proxy URL. Please check your secrets document in this dataset, per the plugin documentation.',
)
}
const url = `https://api.smartling.com/jobs-api/v3/projects/${project}/jobs?jobName=${documentId}`
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The jobName QS filter has been removed because it breaks if the job does not use the documentId as job's name in Smartling.

It fetches all jobs and filter by referenceNumber using the same logic.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hey arthur! i would prefer this stay in the plugin for backwards compatibility. we've only added the additional logic to include the reference number a month or so ago, so other users may have old jobs that do not have a reference number encoded. ideally, this would be removed with a major version change, because it's technically a breaking change for users with in-progress jobs.

there is additional logic in the following few lines to use either the file URI or the reference number as a "catch." if you've noticed breaking behavior with this logic, let me know, but i'd rather catch both cases.

@stipsan stipsan self-assigned this Oct 17, 2023
@cngonzalez
Copy link
Member

this was enabled, I believe! closing now :)

@cngonzalez cngonzalez closed this May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants